Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide option to disable internet connections #774

Merged
merged 2 commits into from Jul 25, 2018
Merged

Conversation

jayhesselberth
Copy link
Collaborator

Closes #773. Closes #762. Closes #766.

@jayhesselberth jayhesselberth merged commit 56f3a65 into master Jul 25, 2018
@jayhesselberth jayhesselberth deleted the internet-opt branch July 25, 2018 18:49
@dongzhuoer
Copy link

Thank you very much for the code, but I'm afraid that there is still one step to get it work.

I set options(pkgdown.internet = F) in my ~/.Rprofile, then

> pkgdown::build_home()
── Building home ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
Reading 'news.md'
Writing 'index.html'
── Previewing site ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
> pkgdown::build_site()
══ Building pkgdown site ═══════════════════════════════════════════════════════════════════════════════════════════════════════════════════
Reading from: '/media/computer/work/R/hgnc'
Writing to:   '/media/computer/work/R/hgnc/docs'
── Initialising site ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
Writing 'sitemap.xml'Edit '_pkgdown.yml'
── Building home ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
Reading 'news.md'
Error in curl::curl_fetch_memory(url, handle = handle) : 
  Could not resolve host: cloud.r-project.org
Calls: withCallingHandlers ... request_fetch.write_memory -> <Anonymous> -> .handleSimpleError -> h
Execution halted
Error in curl::curl_fetch_memory(url, handle = handle) : 
  Could not resolve host: cloud.r-project.org

@dongzhuoer
Copy link

dongzhuoer commented Jul 27, 2018

I found that build_site() use new_process = TRUE by default.

In this scenario, I think it's quite intuitive to enable .Rprofile in the new process or at least add option to let the user explicitly specify it.

Otherwise, user have to run pkgdown::build_site(new_process = F) (I don't think it's a good idea to have to run pkgdown in a dirty process in order to get pkgdown.internet working)

@jayhesselberth
Copy link
Collaborator Author

I think we need to add user_profile = TRUE to the callr::r() call.

But then you have to set the option via .Rprofile.

@dongzhuoer
Copy link

Yeah, I think it's the natural way.

@hadley
Copy link
Member

hadley commented Jul 27, 2018

I think it would be better to set the option explicitly in the new process

@dongzhuoer
Copy link

dongzhuoer commented Jul 28, 2018

How can you achieve that? build_site(process_setup = function() {options(pkgdown.internet = F)})? I think the syntax is ugly.

Instead .Rprofile is the natural way to run set up code before a R process begins. we can use build_site(use_Rprofile = TRUE).

The only drawback is when user want to run some code in build_site(new_process = T), but not in build_site(new_process = F). However, in normal case, I think this need is rare.

@hadley
Copy link
Member

hadley commented Jul 28, 2018

pkgdown would do it for you.

Writing to .Rprofile is not a good solution.

@dongzhuoer
Copy link

dongzhuoer commented Jul 29, 2018

If the user develops multiple packages, specify that in every build_site() is rather tedious. .Rprofile is a much better choice.

Another way is to set it in _pkgdown.yml, like what I proposed in #762 (comment)

@dongzhuoer
Copy link

Maybe we can have something like

setup:
  code: options(pkgdown.internet = F)

or

setup:
  options:
    pkgdown.internet: F

@petermeissner
Copy link

petermeissner commented Jul 31, 2018

👍 For me the builds now run through with:

options(pkgdown.internet = FALSE)
pkgdown::buil_site(new_process = FALSE)

But there are more problems ahead. Since internet is no option looking at the docs in a browser will result in several files the browser cannot obtain:

  • jquery-3.1.0.min.js
  • bootstrap.min.css
  • bootstrap.min.js
  • font-awesome.min.css
  • clipboard.min.js
  • sticky-kit.js
  • Math.Jax.js?config=Tex-AMS-MML_HTMLorMML

@dongzhuoer
Copy link

@petermeissner I know this way, the down point is that we have to run pkgdown in a dirty session.

@dongzhuoer
Copy link

dongzhuoer commented Aug 1, 2018

I think we might have gone so far.

What I'm talking about originally is to disable internet for building the documentation, not for viewing the documentation.

Maybe we need separate option for these two senarios.

@BruceZhaoR
Copy link

@jayhesselberth 56f3a65#r30192460

https://cran.r-project.org/package= may be better than https://cloud.r-project.org/package= ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants